Fix panic on a rare startup race condition#8390
Closed
jimmygchen wants to merge 2 commits intosigp:unstablefrom
Closed
Fix panic on a rare startup race condition#8390jimmygchen wants to merge 2 commits intosigp:unstablefrom
jimmygchen wants to merge 2 commits intosigp:unstablefrom
Conversation
Member
Author
|
Closing in favour of #8391 |
mergify bot
pushed a commit
that referenced
this pull request
Nov 17, 2025
…8391) Take 2 of #8390. Fixes the race condition properly instead of propagating the error. I think this is a better alternative, and doesn't seem to look that bad. * Lift node id loading or generation from `NetworkService ` startup to the `ClientBuilder`, so that it can be used to compute custody columns for the beacon chain without waiting for Network bootstrap. I've considered and implemented a few alternatives: 1. passing `node_id` to beacon chain builder and compute columns when creating `CustodyContext`. This approach isn't good for separation of concerns and isn't great for testability 2. passing `ordered_custody_groups` to beacon chain. `CustodyContext` only uses this to compute ordered custody columns, so we might as well lift this logic out, so we don't have to do error handling in `CustodyContext` construction. Less tests to update;. Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
A discord user reported a panic during startup and it wasn't recurring. This seems like a rare race condition that wasn't possible until custody backfill was introduced, and this queried custody context before it was initialised:
lighthouse/beacon_node/network/src/sync/custody_backfill_sync/mod.rs
Line 262 in 51ad47f
custody context initialised here:
lighthouse/beacon_node/client/src/builder.rs
Line 488 in 43c5e92
I've been thinking about fixing the race condition but thought this may be a simpler fix, however this end up being quite tedious and requiring a lot of changes.
I'm going to look into the alternatives of fixing the race conditions properly and compare the solutions.
NOTE: This currently targets
unstable, but I can rebase this into the release branch if we want to include this in v8.0.1.